Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ai-brain: thread mapping cache and better formatting of messages #11

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

vaijab
Copy link
Contributor

@vaijab vaijab commented Feb 29, 2024

This is the next iteration in our journey to build a smarter botkube bot. I've added an internal cache to map messaging platform message id (think thread) to an OpenAI thread. The bot will be able to keep conversational context, which drastically improves UX.

I also added a hacky way of converting markdown markup text to slack compatible text markup. Need to somehow distinguish between slack and teams messages. I believe @mszostok has an idea.

Description

Changes proposed in this pull request:

  • ai-brain: implement thread mapping with an internal cache
  • ai-brain: try to convert markdown to slack and teams markup

Here is what it looks like, see screenshots below:
image

Better formatting
image

Related issue(s)

Related https://github.com/kubeshop/botkube-cloud/issues/843

@vaijab vaijab requested a review from a team as a code owner February 29, 2024 05:07
@vaijab vaijab requested a review from pkosiec February 29, 2024 05:07
@vaijab vaijab added the enhancement New feature or request label Feb 29, 2024
Copy link
Collaborator

@mszostok mszostok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested e2e LGTM, great job 🚀 sorry for the long review but wanted to answer your question based on real data, and it took a bit longer that expected 👍

return false, fmt.Errorf("got unexpected status: %s", run.Status)

case openai.RunStatusExpired:
i.cache.Delete(p.MessageID)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be good to send some user facing message:

i.out <- source.Event{
	Message: msgExipredAIAnswer(p.MessageID),
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, I was thinking about that, but an expired thread is by definition old, like 10 or more minutes old, so it could mean that response will be out of the blue and confusing. How about this, I'll make sure we'll add run status as an attribute to honeycomb and then we can see how often it happens, if at all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so side topic, should we even consider polling such runs? maybe we should change the input ctx?
and have sth like:

ctx, cancel := context.WithTimeout(ctx, 5*time.Minute)

so polling will be canceled after 5min (we can then also call CancelRun) and we can respond to the user that we were not able to process this prompt. Otherwise, user will see only:
Screenshot 2024-03-01 at 00 22 15

internal/source/ai-brain/response.go Show resolved Hide resolved
internal/source/ai-brain/response.go Outdated Show resolved Hide resolved
return text
}

// func markdownToTeams(markdownText string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I didn't test that one; I can't tell if the replacement makes sense from the Teams platform perspective

@vaijab vaijab force-pushed the aibrainiterate branch 3 times, most recently from 36c53a4 to 52e3a21 Compare February 29, 2024 14:58
@vaijab vaijab merged commit 2832332 into main Mar 1, 2024
4 checks passed
@vaijab vaijab deleted the aibrainiterate branch March 1, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants